d2ir: Implement lazy globs and triple glob #1552

nhooyr · 2023-08-16T01:42:23Z

This finishes up the globs implementation!

See tests for what I mean by lazy globs and what the triple glob does.

alixander

failed to compile: /Users/alexanderwang/dev/d2-docs/static/bespoke-d2/triple-glob.d2:1:11: fill must be style.fill

***.style.fill: yellow
**.shape: circle
*.style.multiple: true

x: {
  y
}

layers: {
  next: {
    a
  }
}

alixander

applying too many times

# Create a shape `a`
a

* -> y

b
c

alixander

**.child

a
b
c

nhooyr · 2023-08-16T09:18:16Z

Fixed.

alixander

i think globs need to be overridable. currently a is yellow with this script, but it should be red.

**.style.fill: yellow
**.style.fill: red

a

If a parent board has a triple glob, it should also be overrideable by globs in the current board, e.g.

***.style.fill: yellow

layers: {
  hi: {
    **.style.fill: red
    # should be red, but it's yellow right now
    a
  }
}

nhooyr · 2023-08-16T22:20:49Z

Certainly was the intent, not sure how that's happening.

nhooyr · 2023-08-16T23:52:06Z

fixed.

alixander

did u hardcode the word layers in that last fix? i get this err again if i just change to scenarios

**.style.fill: red

scenarios: {
  b: {
    a -> b
  }
}

alixander

(*** -> ***)[*].label: hi

a -> b

layers: {
  hi: {
    (*** -> ***)[*].label: bye

    scenarios: {
      b: {
        # This label is "hi", but it should be "bye"
        a -> b
      }
    }
  }
}

nhooyr · 2023-08-17T01:39:45Z

did u hardcode the word layers in that last fix? i get this err again if i just change to scenarios

nope using d2graph.BoardKeywords so I'm very confused by that too.

addressed

alixander

what do you think about this one?

(*** -> ***)[*].label: hi

# This is "hey" right now but should be "hi"?
a -> b

(*** -> ***)[*].label: hey

interpretation 1: i've defined the first glob and expect all connections to lazily apply it until it's overridden, so the current behavior is broken.

interpretation 2: globs are global. i've defined two globs of the same pattern, so the last one should win, no matter what order of statements come in between/around them, so the current behavior is correct.

i'm leaning to interpretation 1. i.e. Globs should apply backwards unless an identical pattern already has lazily applied to it.

nhooyr · 2023-08-17T16:29:34Z

I think number 2 is the right behavior as it's implemented now. Last one to apply always wins as that's consistent with the rest of the language. Otherwise you can't override/unset globs which isn't good. Especially if you're importing a d2 file you don't control.

Why are you leaning towards 1? Under what kind of scenario do you see this coming up where 1 is preferable?

nhooyr · 2023-08-17T16:32:20Z

lazily apply it until it's overridden, so the current behavior is broken.

But you did override it with the second one?

nhooyr · 2023-08-17T16:33:59Z

i'm leaning to interpretation 1. i.e. Globs should apply backwards unless an identical pattern already has lazily applied to it.

In this case the final glob isn't being applied backwards. It's being applied normally.

alixander

ok, i can see that. moving on, this one's a bug for sure:

(*** -> ***)[*].label: hi

a -> b: {
  label: bye
}

nhooyr · 2023-08-17T18:09:38Z

🤦🏼 another good catch

alixander

looks like with filters, it's not overriding

x

*.style.opacity: 0.1
*: {
  &label: x
  style.opacity: 1
}

nhooyr · 2023-08-17T21:10:32Z

First one fixed, working on this next one.

nhooyr · 2023-08-17T21:14:30Z

Also btw #1552 (review) was because of *** being changed to **, not because of layers -> scenarios.

And I think this next one isn't related to globs but that label isn't handled specially by filters to match the ID of a field without a label.

alixander · 2023-08-17T21:32:33Z

@nhooyr

nah i just tried this and it's the same issue

*.style.opacity: 0.1
*: {
  &label: x
  style.opacity: 1
}

x: x

nhooyr · 2023-08-17T21:48:24Z

Same issue there, label needs special handling. Pushed fix for *Field but I think I need to add the same exception for *Edge too. So hold on.

nhooyr · 2023-08-17T21:54:00Z

Fixed.

On query globs, filtering without the edge index overfilters as we only match one of the edge instead all of them.

alixander

is it easy to get globs working for imports? right now this doesn't work but i definitely see the utility

rules.d2

*.style.fill: red

index.d2

...@rules
x

nhooyr · 2023-08-30T11:43:54Z

Right now no unfortunately. Would require changing how imports work to merge ASTs together instead of IRs.

alixander · 2023-08-30T11:53:43Z

i think that can be another use case of ***. they cross board boundaries and file boundaries

alixander · 2023-08-30T11:55:10Z

can we encode globs into the IR? i think imports should continue just dealing with IRs, globs just need to be in there

nhooyr · 2023-08-30T11:56:53Z

Perhaps, I'm not sure of a good way to do it right now.

alixander

the fill isn't applying, but if I remove the x, it works. Looks like something straightforward missed?

(left is index)

nhooyr requested a review from alixander August 16, 2023 01:42

alixander requested changes Aug 16, 2023

View reviewed changes

nhooyr requested a review from alixander August 16, 2023 06:35

alixander requested changes Aug 16, 2023

View reviewed changes

nhooyr requested a review from alixander August 16, 2023 23:52

alixander requested changes Aug 17, 2023

View reviewed changes

alixander previously requested changes Aug 17, 2023

View reviewed changes

nhooyr requested a review from alixander August 17, 2023 05:49

alixander reviewed Aug 17, 2023

View reviewed changes

alixander requested changes Aug 17, 2023

View reviewed changes

nhooyr force-pushed the lazy-globs branch from 7269541 to aa2be81 Compare August 17, 2023 21:12

nhooyr force-pushed the lazy-globs branch from e848eed to 317964a Compare August 17, 2023 21:46

nhooyr requested a review from alixander August 17, 2023 21:54

nhooyr added 8 commits August 30, 2023 00:37

d2ir: Add exception for &label filter

1800ae3

d2ir: Add label filter exception for edges too

2efd87f

d2ir: Remove stale testdata

69df741

d2ir: Filter glob creation of edges with edge index where appropriate

57ffc8d

On query globs, filtering without the edge index overfilters as we only match one of the edge instead all of them.

d2ir: Many more glob fixes

eed3f7e

d2ir: More alex fixes

47d6712

make.sh: Enforce at least go 1.20 instead of exactly go 1.20

df25d8b

d2ir: Ensure filters pass before setting primary

88b885a

nhooyr force-pushed the lazy-globs branch from d9cea18 to 88b885a Compare August 30, 2023 07:37

d2ir: Ensure new scenario map doesn't re overlay

4523b50

alixander requested changes Aug 30, 2023

View reviewed changes

nhooyr force-pushed the lazy-globs branch from 03dc5a3 to 8782a7d Compare August 30, 2023 12:11

nhooyr requested a review from alixander August 30, 2023 12:16

d2ir: Fix Alex's null glob problem

d03082d

nhooyr force-pushed the lazy-globs branch 2 times, most recently from 303058d to 1d9f0aa Compare September 4, 2023 05:52

d2ir: Make globs work through imports

b1f9083

nhooyr force-pushed the lazy-globs branch from 1d9f0aa to b1f9083 Compare September 4, 2023 06:18

alixander reviewed Sep 4, 2023

View reviewed changes

nhooyr requested a review from alixander September 4, 2023 22:02

d2ir: Fixup lazy globs across imports

64c2c84

nhooyr force-pushed the lazy-globs branch from 5ac1db3 to 64c2c84 Compare September 4, 2023 22:29

alixander approved these changes Sep 5, 2023

View reviewed changes

nhooyr merged commit dd131f8 into terrastruct:master Sep 5, 2023
2 checks passed

nhooyr deleted the lazy-globs branch September 5, 2023 09:03

Porkepix mentioned this pull request Sep 27, 2023

d2 0.6.1 Homebrew/homebrew-core#146218

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

d2ir: Implement lazy globs and triple glob #1552

d2ir: Implement lazy globs and triple glob #1552

nhooyr commented Aug 16, 2023

alixander left a comment

alixander left a comment •

edited

Loading

alixander left a comment •

edited

Loading

nhooyr commented Aug 16, 2023

alixander left a comment •

edited

Loading

nhooyr commented Aug 16, 2023

nhooyr commented Aug 16, 2023

alixander left a comment

alixander left a comment

nhooyr commented Aug 17, 2023 •

edited

Loading

alixander left a comment •

edited

Loading

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

alixander left a comment

nhooyr commented Aug 17, 2023

alixander left a comment

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

alixander commented Aug 17, 2023

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

alixander left a comment •

edited

Loading

nhooyr commented Aug 30, 2023

alixander commented Aug 30, 2023

alixander commented Aug 30, 2023

nhooyr commented Aug 30, 2023

alixander left a comment •

edited

Loading

d2ir: Implement lazy globs and triple glob #1552

d2ir: Implement lazy globs and triple glob #1552

Conversation

nhooyr commented Aug 16, 2023

alixander left a comment

Choose a reason for hiding this comment

alixander left a comment • edited Loading

Choose a reason for hiding this comment

alixander left a comment • edited Loading

Choose a reason for hiding this comment

nhooyr commented Aug 16, 2023

alixander left a comment • edited Loading

Choose a reason for hiding this comment

nhooyr commented Aug 16, 2023

nhooyr commented Aug 16, 2023

alixander left a comment

Choose a reason for hiding this comment

alixander left a comment

Choose a reason for hiding this comment

nhooyr commented Aug 17, 2023 • edited Loading

alixander left a comment • edited Loading

Choose a reason for hiding this comment

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

alixander left a comment

Choose a reason for hiding this comment

nhooyr commented Aug 17, 2023

alixander left a comment

Choose a reason for hiding this comment

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

alixander commented Aug 17, 2023

nhooyr commented Aug 17, 2023

nhooyr commented Aug 17, 2023

alixander left a comment • edited Loading

Choose a reason for hiding this comment

rules.d2

index.d2

nhooyr commented Aug 30, 2023

alixander commented Aug 30, 2023

alixander commented Aug 30, 2023

nhooyr commented Aug 30, 2023

alixander left a comment • edited Loading

Choose a reason for hiding this comment

alixander left a comment •

edited

Loading

alixander left a comment •

edited

Loading

alixander left a comment •

edited

Loading

nhooyr commented Aug 17, 2023 •

edited

Loading

alixander left a comment •

edited

Loading

alixander left a comment •

edited

Loading

alixander left a comment •

edited

Loading